Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge dev into tokio_migration and fix warnings #683

Closed

Conversation

Johannesd3
Copy link
Contributor

... and fix (new?) clippy lints. Clippy didn't like the naming of some types and enum variants, consecutive upper letters are probably reserved for constants.

Maybe it's possible to fast-forward this merge to avoid an additional (useless) merge commit, but I'm no github pr expert.

@Johannesd3 Johannesd3 changed the base branch from tokio_migration to dev April 1, 2021 16:04
@Johannesd3 Johannesd3 changed the base branch from dev to tokio_migration April 1, 2021 16:04
@Johannesd3 Johannesd3 changed the title Merge dev into tokio_migration Merge dev into tokio_migration and fix tests Apr 1, 2021
@Johannesd3
Copy link
Contributor Author

Is it too much CI @sashahilton00 ?

@Johannesd3 Johannesd3 mentioned this pull request Apr 5, 2021
13 tasks
@sashahilton00
Copy link
Member

@Johannesd3 it looks fine. My only concern is whether the tests on the various different systems are run in parallel and ho long they take, not sure how much resource Github grants projects to run CI workloads.

@Johannesd3
Copy link
Contributor Author

Johannesd3 commented Apr 9, 2021

No, that's too much. It's 100 minutes per run, and I think that's 20 runs per month, that's not enough. (I don't know why mac os suddenly failed.)

@kingosticks
Copy link
Contributor

Github actions is free for public repos, right? It's only private repos that have the 2k monthly limit.

@Johannesd3
Copy link
Contributor Author

Didn't know that, thx. Another inconvenience: It will take twice as long (15min) until the tests are finished ...

I will try to rebase this PR using the latest changes from dev, and try to fix the macOS problem

@Johannesd3
Copy link
Contributor Author

The target directory should rather be cached by os + rustc version, but I have no idea how to achieve this.

Anyway, I will close this PR and reopen it, as I want to see how much caching improves the speed.

@Johannesd3 Johannesd3 closed this Apr 9, 2021
@Johannesd3 Johannesd3 reopened this Apr 9, 2021
@kingosticks
Copy link
Contributor

Any idea why the Run cargo test --workspace step after Run cargo build --workspace --examples does more compilation, including some of the same stuff again? Is that right?

@Johannesd3
Copy link
Contributor Author

Yes, those #[cfg(test)] blocks are only considered if the tests are executed, but not if the examples are compiled, so every crate needs to be compiled again.

@Johannesd3
Copy link
Contributor Author

Next try to check caching again, hopefully it's the last try. Sorry for spamming you.

@Johannesd3 Johannesd3 closed this Apr 9, 2021
@Johannesd3 Johannesd3 reopened this Apr 9, 2021
@Johannesd3
Copy link
Contributor Author

I don't quite understand the macOS error :( Apparently it's caused by caching, but I don't know why.

@Johannesd3 Johannesd3 changed the title Merge dev into tokio_migration and fix tests Merge dev into tokio_migration and fix warnings Apr 9, 2021
@Johannesd3
Copy link
Contributor Author

I reduced the scope of this PR to experiment further with CI without blocking this. I created a PR in my repository (Johannesd3#13) where I can test stuff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants